Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

don't fetch indices , add indices to cache, and add esmonthly func #1931

Merged
merged 2 commits into from
Oct 20, 2016
Merged

don't fetch indices , add indices to cache, and add esmonthly func #1931

merged 2 commits into from
Oct 20, 2016

Conversation

kylebrandt
Copy link
Member

No description provided.

@kylebrandt
Copy link
Member Author

Currently if the time series is missing it errors inside ESDateHistrogram when it can't get the expected time series in the result. This result gets cached. Before when it would error on no indices in getFn, a wrapper for func (e ElasticHosts) Query(r *ElasticRequest) (*elastic.SearchResult, error), ensuring it was not cached. But now a lack of indices throws an error. Need to research inspecting the shards of the result and understanding what the means in this context.

@kylebrandt
Copy link
Member Author

I think counting the resulting shards and erroring when there were no shards that matched will get the result I want.

However, it depends on the following PR olivere/elastic#373

Alternatively, I could go for the date math method mentioned in #1926 (comment) . But only if allow_no_indices applies to datemath and not just wildcard. According to the documentation it only applies to wildcard which I haven't tested yet. Even if does work I think the implementation will be a bit of pain.

@kylebrandt
Copy link
Member Author

note: forgot to fmt, force pushed the fmt.

@kylebrandt kylebrandt changed the title WIP: don't fetch indices , add indices to cache don't fetch indices , add indices to cache, and add esmonthly func Oct 20, 2016
captncraig
captncraig approved these changes Oct 20, 2016
View reviewed changes
cmd/bosun/expr/elastic.go
}
return str, nil
return fmt.Sprintf("%v\n%s", r.Indices, b), nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably use string(b) here. Don't want [42 31 13] in you key.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

additionally, what does the source have that causes it to be a different key? What varies?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ask because it surprises me that something like getting a cache key even has a possibility of erroring.

@kylebrandt
Copy link
Member Author

fixes #1926

@kylebrandt kylebrandt merged commit 3438de5 into master Oct 20, 2016
@gbrayut gbrayut deleted the esNoIndexRefactor branch March 1, 2017 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@captncraig captncraig captncraig approved these changes

Assignees

@captncraig captncraig

Labels
PR - Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kylebrandt @captncraig